fix(desktop): set MakerDeb/MakerRpm bin to lightfast#652
Merged
jeevanpillay merged 1 commit intomainfrom May 7, 2026
Merged
Conversation
The Linux makers default `options.bin` to `pkg.name` (via electron-installer-common's getDefaultsFromPackageJSON). After the release workflow runs `npm version`, that value is `@lightfast/desktop` — the package's npm name, not the executable name. The actual binary on disk is `lightfast`, set via `packagerConfig.executableName`. Without an explicit override, both makers fail at the binary-symlink step: could not find the Electron app binary at ".../Lightfast-linux-x64/@lightfast/desktop" Set `options.bin: "lightfast"` on both MakerDeb and MakerRpm. Surfaced by the @lightfast/desktop@0.1.0-rc.5 dry-run; mac legs were unaffected.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Contributor
📝 WalkthroughWalkthroughExplicit ChangesLinux Binary Packaging
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/forge.config.ts (1)
164-164: ⚡ Quick win[warning] Single-source the executable name to avoid future drift.
"lightfast"is now repeated across packager and makers (Line 116, Line 164, Line 181). Extract a constant so future renames don’t silently reintroduce this class of release break.Proposed diff
+const EXECUTABLE_NAME = "lightfast"; + const config: ForgeConfig = { packagerConfig: { name: "Lightfast", - executableName: "lightfast", + executableName: EXECUTABLE_NAME, @@ // .../Lightfast-linux-<arch>/@lightfast/desktop". - bin: "lightfast", + bin: EXECUTABLE_NAME, @@ // shares the same `getDefaultsFromPackageJSON` helper. - bin: "lightfast", + bin: EXECUTABLE_NAME,Also applies to: 181-181
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/forge.config.ts` at line 164, The executable name "lightfast" is hardcoded in multiple places (e.g., the bin field and packager/makers) which can drift; introduce a single constant (e.g., APP_NAME or EXECUTABLE_NAME) at the top of this module and replace all literal occurrences (the bin: "lightfast" entry and the repeated names referenced around the packager and makers blocks) with that constant so renames only require one change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/desktop/forge.config.ts`:
- Line 164: The executable name "lightfast" is hardcoded in multiple places
(e.g., the bin field and packager/makers) which can drift; introduce a single
constant (e.g., APP_NAME or EXECUTABLE_NAME) at the top of this module and
replace all literal occurrences (the bin: "lightfast" entry and the repeated
names referenced around the packager and makers blocks) with that constant so
renames only require one change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 539332c0-59ae-4a76-9b5e-72f4a4d86e58
📒 Files selected for processing (1)
apps/desktop/forge.config.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Surfaced by the
@lightfast/desktop@0.1.0-rc.5dry-run after PR #650 merged: bothBuild Linux x64andBuild Linux arm64legs failed identically at the maker step:Root cause
electron-installer-common'sgetDefaultsFromPackageJSONdefaultsoptions.bintopkg.name. After the release workflow runsnpm version, that field is@lightfast/desktop(the npm package name) — not the executable name. The actual binary on disk islightfast, set viapackagerConfig.executableNameinforge.config.ts:114. The makers don't readexecutableName— they have their ownbinoption which I didn't set in PR #650.MakerRpmshares the same default viaelectron-installer-redhat, so both makers fail the same way.Fix
Set
options.bin: "lightfast"on bothMakerDebandMakerRpm(one line each + a comment explaining the override). No other changes.Why local builds missed this
pnpm make --platform=darwin --arch=arm64(the Phase 1 verification) only exercises mac makers. Local Linux make wasn't in the verification path; the rc dry-run was the integration test for "Linux release path produces installable artifacts" — pattern fromfeedback_release_pipeline_dryrun.md(rc.1 → rc.4 surfaced 7 bugs invisible to local builds; this is bug #1 of the rc.5 round).Test plan
pnpm --filter @lightfast/desktop typecheckpasses locallymacos-14+ubuntu-22.04) goes green on this PR@lightfast/desktop@0.1.0-rc.6and confirm:build_linuxarch legs go greenlightfast_<v>_amd64.deb,lightfast_<v>_arm64.deb,lightfast-<v>.x86_64.rpm,lightfast-<v>.aarch64.rpmfinalizejob runs (was skipped in rc.5).dmgattestation glob extension still works (PR feat(desktop): linux makers + ci matrix; also extend mac attestation to .dmg #650 bundled change)Notes for reviewer
The mac legs in rc.5 succeeded — confirms PR #650's
build→build_macosrename and the.dmgattestation extension don't regress. The only failure was the Linux makerbindefault, which this PR fixes.Summary by CodeRabbit